Skip to content

Draft: Implement watching file paths via oslib.watch #5068

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 27 commits into from
May 19, 2025

Conversation

arturaz
Copy link
Contributor

@arturaz arturaz commented May 5, 2025

Currently watching files uses polling which is inefficient with large codebases. On my laptop ./mill --watch uses 20% CPU just to watch the files for changes.

This uses oslib's watch module to watch for the file changes instead of polling them.

We had to update oslib (com-lihaoyi/os-lib#386) to:

  • add a capability of filtering what folders it watches. This is needed because watching large generated folders like out or .bloop emits Java FS watcher OVERFLOW events.
  • remove the random println's that oslib.watch had.

The current approach to watching is to watch the workspace root via FS watching. FS watching only works with folders and we have to watch root/build.mill, and thus, root/, so everything else falls under it.

Mac OS watcher performs watching recursively natively, but on linux oslib adds recursive watches itself, so we use oslib.watch filter to prevent watching anything that is not an ancestor of watched root. For example, if we have source at root/module-a/src/, we'll watch root/, root/module-a/ and root/.

Finally, events are filtered so that unrelated changes (like creating root/random-file.txt) does not trigger reevaluation.

The fs watching is enabled by default and can be disabled via --notify-watch=false.

0.12.x version: #5073

@lefou
Copy link
Member

lefou commented May 6, 2025

What are the CPU numbers on your laptop with this change applied?

@arturaz
Copy link
Contributor Author

arturaz commented May 6, 2025

What are the CPU numbers on your laptop with this change applied?

I haven't tested it yet, as my repo uses mill 0.12.10. With #5073 I'll be able to test it and report it later.

@arturaz
Copy link
Contributor Author

arturaz commented May 7, 2025

What are the CPU numbers on your laptop with this change applied?

I tested the 0.12.x branch, the CPU now hover's around 0.1%-2.5%, not enough to trigger the CPU fan, whilst previously CPU fan was constantly on.

Screencast_20250507_200300.webm

@arturaz arturaz force-pushed the improvement/fs-watching branch from 95c753d to 3e7c22c Compare May 8, 2025 09:56
@lihaoyi lihaoyi marked this pull request as draft May 13, 2025 05:26
@arturaz arturaz force-pushed the improvement/fs-watching branch from a2d3e4b to 73b6b6f Compare May 13, 2025 06:28
@arturaz arturaz force-pushed the improvement/fs-watching branch from 3ef16e8 to 1b797e9 Compare May 13, 2025 06:45
@lihaoyi lihaoyi marked this pull request as ready for review May 19, 2025 08:36
arturaz and others added 3 commits May 19, 2025 11:49
…hing

# Conflicts:
#	integration/invalidation/watch-source-input/src/WatchSourceInputTests.scala
#	libs/main/src/mill/main/MainModule.scala
#	runner/daemon/src/mill/daemon/MillMain.scala
@lihaoyi lihaoyi merged commit 6ca941d into com-lihaoyi:main May 19, 2025
37 of 38 checks passed
@lefou lefou added this to the 1.0.0-RC1 milestone May 19, 2025
lihaoyi pushed a commit that referenced this pull request May 19, 2025
…branch) (#5073)

Backport of #5068

The fs watching is *disabled* by default and can be disabled via
`--watch-via-fs-notify=true`.

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
@lihaoyi lihaoyi mentioned this pull request May 19, 2025
lihaoyi added a commit that referenced this pull request May 20, 2025
Pulls in @arturaz's notify `--watch` changes
(#5068) and runBackground
changes (#5115) for dogfooding
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants